Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch data representation in python wrapper #53

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

a-sevin
Copy link
Member

@a-sevin a-sevin commented Aug 4, 2023

From column-major order to row-major order

It should fix the issue #22 and remove the transpose in pyMilk

From column-major order to row-major order
@a-sevin a-sevin linked an issue Aug 4, 2023 that may be closed by this pull request
@a-sevin a-sevin requested a review from DasVinch August 4, 2023 13:57
@joseph-long
Copy link
Contributor

My memory did not deceive me... a fix exists for #22! But it never got merged. Is there anything blocking it?

@DasVinch
Copy link
Member

I've never merged this because I was never too sure of the implications with FITS files and with pyMilk, precisely what C/F style assumptions are made in which layer and how this PR relaxes them (basically what I was saying in #22, and I never really dug deeper). I guess as long as we can maintain guarantees between python and ISIO, we should be good.
But I'm sure this would require a thorough pyMilk inspection and/or testing.

However, #22 has been fixed at the pyMilk level at the cost of a copy in the case that was causing the bug.

See https://github.com/milk-org/pyMilk/blob/a73f72ae83f30dc6ed4fd22e397466b0f01348b0/pyMilk/interfacing/shm.py#L683

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImageStreamIOWrap in Python fails on C-order arrays in confusing way
3 participants